Add Service Account ID validation - #6115
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the configuration validation framework by adding a dedicated validator for GCP Service Account IDs. It ensures that generated service account names comply with GCP's strict naming rules, preventing invalid configurations from reaching the deployment stage. The changes include the core logic, registration of the new validator, and comprehensive test coverage to ensure reliability. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![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 a new service_account_id validator to verify that the combination of deployment_name and name forms a valid GCP Service Account ID. It includes the validator implementation, registration, and comprehensive unit tests. Feedback on the changes focuses on preventing potential panics by checking if the resolved values are known and not null before calling AsString(), as well as optimizing performance by compiling the regular expression once at the package level instead of on every validation call.
- Avoid panics by checking cty.Value.IsKnown() and cty.Value.IsNull() before calling AsString() on resolved settings. - Optimize by compiling the Service Account ID regular expression once at the package level. TAG=agy CONV=433bb778-4af5-49a0-aac2-fac0e962fdce
Limit the base example name to 8 characters when constructing the DEPLOYMENT name. This prevents the combined Service Account ID (deployment name + SA name) from exceeding GCP's 30-character limit during integration tests. TAG=agy CONV=433bb778-4af5-49a0-aac2-fac0e962fdce
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a concatenation feature to the regex validator, allowing multiple settings to be joined and validated together. This is applied to the service-account module to validate the combined service account ID. Feedback highlights a potential panic on cty.NilVal and a validation bypass issue when relying on default values in resolveSettingToString. Additionally, a simplification is suggested for generating the short suffix in the configuration validation script.
2 participants
This PR validates the combined length and format of the deployment name and service account name to ensure they comply with Google Cloud's 30-character limit and naming rules. This validation runs during the pre-flight phase, preventing late-stage deployment failures in Terraform.
The validator is safe-guarded against null or unknown values to prevent panics during blueprint expansion, and uses a pre-compiled regular expression for efficiency.
Submission Checklist
NOTE: Community submissions can take up to 2 weeks to be reviewed.
Please take the following actions before submitting this pull request.