feat(cfg): setup config.proto code generation and privacy scrubber - #4958
feat(cfg): setup config.proto code generation and privacy scrubber#4958PranjalC100 wants to merge 4 commits into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces automated generation of a Protocol Buffers configuration schema (config.proto) by updating the configuration generator tool (tools/config-gen). It maps configuration parameters to proto types, assigns sequential tags, and generates the proto file using a new template. Feedback on this PR suggests using the leaf segment of the configuration path instead of the full flag name to prevent redundant field names in nested proto messages (e.g., FileCacheConfig.file_cache_max_size_mb). Additionally, it is recommended to use filepath.Join instead of path.Join in the generator tool to ensure cross-platform compatibility.
4b89270 to
61c7a48
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4958 +/- ##
===========================================
+ Coverage 0 83.80% +83.80%
===========================================
Files 0 173 +173
Lines 0 21334 +21334
===========================================
+ Hits 0 17879 +17879
- Misses 0 2775 +2775
- Partials 0 680 +680
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
0b7b051 to
1755c75
Compare
github-actions
Bot
added
the
remind-reviewers
|
Hi @kislaykishore, @meet2mky, your feedback is needed to move this pull request forward. This automated reminder was triggered because there has been no activity for over 24 hours. Please provide your input when you have a moment. Thank you! |
3 similar comments
|
Hi @kislaykishore, @meet2mky, your feedback is needed to move this pull request forward. This automated reminder was triggered because there has been no activity for over 24 hours. Please provide your input when you have a moment. Thank you! |
|
Hi @kislaykishore, @meet2mky, your feedback is needed to move this pull request forward. This automated reminder was triggered because there has been no activity for over 24 hours. Please provide your input when you have a moment. Thank you! |
|
Hi @kislaykishore, @meet2mky, your feedback is needed to move this pull request forward. This automated reminder was triggered because there has been no activity for over 24 hours. Please provide your input when you have a moment. Thank you! |
PranjalC100
added
execute-integration-tests
|
Hi @kislaykishore, @meet2mky, your feedback is needed to move this pull request forward. This automated reminder was triggered because there has been no activity for over 24 hours. Please provide your input when you have a moment. Thank you! |
This commit establishes the foundational schema infrastructure by introducing automated Protobuf schema generation and a privacy scrubber for high-risk text fields. File-by-file changes: - cfg/config.proto: The generated Protobuf definition file derived from params.yaml, establishing the single source of truth for the telemetry payload. - tools/config-gen/main.go: Refactored the generator output loop to support compiling .proto files. - tools/config-gen/parser.go: Added populateProtoMetadata mapping logic to convert native Go types into Protobuf equivalents. Also introduced the implicit privacy scrubber that automatically transforms string/path fields into safe is_XYZ_set boolean flags. - tools/config-gen/type_template_data_gen.go: Extended the fieldInfo structures to expose the proto metadata to templates and dynamically inject sequential Protobuf field tags. - tools/config-gen/templates/config_proto.tpl: Added the text/template directive to correctly format the output into proto3 syntax. - tools/config-gen/parser_test.go: Added TestPopulateProtoMetadataExhaustive to exhaustively verify that every config type is successfully mapped to its Protobuf type and privacy conditions are met.
1755c75 to
f474beb
Compare
f474beb to
9d01089
Compare
|
Hi @meet2mky, @kislaykishore, your feedback is needed to move this pull request forward. This automated reminder was triggered because there has been no activity for over 24 hours. Please provide your input when you have a moment. Thank you! |
4 similar comments
|
Hi @meet2mky, @kislaykishore, your feedback is needed to move this pull request forward. This automated reminder was triggered because there has been no activity for over 24 hours. Please provide your input when you have a moment. Thank you! |
|
Hi @meet2mky, @kislaykishore, your feedback is needed to move this pull request forward. This automated reminder was triggered because there has been no activity for over 24 hours. Please provide your input when you have a moment. Thank you! |
|
Hi @meet2mky, @kislaykishore, your feedback is needed to move this pull request forward. This automated reminder was triggered because there has been no activity for over 24 hours. Please provide your input when you have a moment. Thank you! |
|
Hi @meet2mky, @kislaykishore, your feedback is needed to move this pull request forward. This automated reminder was triggered because there has been no activity for over 24 hours. Please provide your input when you have a moment. Thank you! |
| if param.FlagName == "app-name" { | ||
| continue |
There was a problem hiding this comment.
Let's not hardcode it. It's okay to send an extra param to keep the code and analysis less complex.
| option go_package = "github.com/googlecloudplatform/gcsfuse/v3/cfg/pb"; | ||
|
|
||
|
|
||
| message CloudProfilerConfig { |
There was a problem hiding this comment.
The tag numbers require old params.yaml to be present and cannot be generated from scratch later on. This makes the design brittle. I'd rather we hardcode the tag numbers in the params.
|
Hi @meet2mky, your feedback is needed to move this pull request forward. This automated reminder was triggered because there has been no activity for over 24 hours. Please provide your input when you have a moment. Thank you! |
2 participants
Description
This pull request establishes the foundational schema infrastructure by automatically generating the
cfg/config.protodefinition directly fromcfg/params.yaml, which acts as the single source of truth for the telemetry payload.Key Features:
config_proto.tpland a compilation loop intools/config-gen/main.goto auto-generate the schema with sequential field tags.config.protofile to extract and re-use previously assigned field tags, and automatically calculatesmaxTag + 1to assign to any entirely new configuration parameters.parser.gomaps high-risk text types (string, resolvedPath, []string) into safe boolean indicators (e.g.,is_app_name_set).File changes:
cfg/config.proto: The auto-generated Protobuf definition file derived from the configuration source.tools/config-gen/main.go: Refactored the generator output loop to support.protofile compilation.tools/config-gen/parser.go: Implements thepopulateProtoMetadatamapping and the privacy scrubbing logic for high-risk types.tools/config-gen/type_template_data_gen.go: Parses the existingconfig.prototo build a tag registry, exposing proto metadata and dynamically assigning stable backward-compatible field tags into the templates.tools/config-gen/templates/config_proto.tpl: A newtext/templatedirective to format the output intoproto3syntax.Testing details
Manually verified the generated sequential tags and privacy nomenclatures. Added exhaustive unit tests in
parser_test.goandtype_template_data_gen_test.goto validate both the privacy logic and the backward-compatible tag assignment.Backward Compatibility
Enforced by design. Older flag definitions (even deprecated ones) retain their explicit tags to prevent tag collisions and schema breaks in legacy GCSFuse versions.