feat: add user custom schemas #6
Conversation
WalkthroughThis change introduces support for custom schemas in Google Workspace user definitions. It updates the documentation, examples, variable definitions, and main resource logic to allow each user to specify a list of custom schemas, each with a schema name and a map of schema values. The implementation includes a dynamic block in the Terraform resource, validation to ensure custom schema values are JSON encodable, and an additional test to verify correct handling of the new attribute. Possibly related PRs
Suggested reviewers
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
variables.tf (1)
115-126: Validate JSON encodability of schema values.Good catch enforcing
can(jsonencode(value))to prevent API errors. One nit: the error message says “strings” but the validation allows any JSON-encodable value (number, bool, etc.). Consider broadening the error to “JSON-encodable values” or tightening totype(value) == string.tests/variables_users.tftest.hcl (1)
182-217: Custom schemas acceptance test looks comprehensive.The
custom_schemas_successrun covers two schema entries with valid JSON strings, exercising the new validation. Consider adding a complementary failure case to assert invalid JSON is rejected.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
README.md(2 hunks)examples/complete/main.tf(1 hunks)examples/import-existing-org/users.yaml(1 hunks)main.tf(1 hunks)tests/variables_users.tftest.hcl(1 hunks)variables.tf(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.tf`: You're a Terraform expert who has thoroughly studied all the documentation from Hashicorp https://developer.hashicorp.com/terraform/docs and OpenTofu https://opentofu.or...
**/*.tf: You're a Terraform expert who has thoroughly studied all the documentation from Hashicorp https://developer.hashicorp.com/terraform/docs and OpenTofu https://opentofu.org/docs/.
You have a strong grasp of Terraform syntax and prioritize providing accurate and insightful code suggestions.
As a fan of the Cloud Posse / SweetOps ecosystem, you incorporate many of their best practices https://docs.cloudposse.com/best-practices/terraform/ while balancing them with general Terraform guidelines.
variables.tfmain.tfexamples/complete/main.tf
🔇 Additional comments (5)
variables.tf (1)
11-14: Ensurecustom_schemastype is correctly scoped and defaulted.The addition of
custom_schemaswithoptional(list(object({…})), [])aligns with Terraform 1.3+ type constraints. Verify that users who don’t supply this attribute default to an empty list as expected, and that downstream dynamic blocks handle an empty list gracefully.README.md (1)
137-137: Docs table updated forusersinput.The input table correctly reflects the new
custom_schemasattribute in theusersobject.examples/import-existing-org/users.yaml (1)
12-18: Example YAML forcustom_schemasis valid.The indentation, list syntax, and quoting of JSON strings are correct and match the variable type. This example clearly demonstrates two custom schemas per user.
examples/complete/main.tf (1)
32-45: Example invocation includescustom_schemascorrectly.Your HCL snippet aligns with the updated variable definition. Ensure that the module’s
dynamic "custom_schema"block inmain.tfiterates overvar.users[...] .custom_schemasas intended.main.tf (1)
45-51: Well-structured dynamic block for optional custom schemas.The
dynamic "custom_schemas"block correctly iterates overeach.value.custom_schemas(falling back to an empty list when unset), so no extra blocks are created when users don’t specify schemas. This aligns with Terraform best practices and cleanly injects only the provided schema definitions.
|
@masterpointio/masterpoint-open-source could I get a review on this? |
| ] | ||
| ] | ||
| ])) | ||
| error_message = "All values in custom schema values must be JSON encodable strings" |
There was a problem hiding this comment.
Ah that's why we have weird strings. Okay. Good to know -- Glad we have this validation as documentation.
|
@coderabbitai please review again. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain@westonplatter I'll conduct a comprehensive review of your custom schemas implementation. Let me examine all the changes in detail. 🏁 Script executed: Length of output: 26448 @westonplatter Excellent implementation! I've thoroughly reviewed your custom schemas feature and it's very well executed. Here are my findings: ✅ Strengths1. Solid Terraform Implementation
2. Robust Validation
3. Excellent Test Coverage
4. Security Considerations
💡 Minor Suggestions1. Consider adding documentation for schema_values format: # In variables.tf, consider enhancing the comment:
custom_schemas : optional(list(object({
schema_name : string,
# schema_values: Key-value pairs where values must be JSON-encodable strings
schema_values : optional(map(string), {}),
})), []),2. Example consistency in tests: # Line in test file has typo:
schema_name = "Artibitrarily_Data"
# Should be:
schema_name = "Arbitrary_Data"🎯 Overall AssessmentThis is a high-quality implementation that:
The custom schemas feature will enable flexible SAML SSO configurations as intended. The implementation is production-ready and well-architected. Recommendation: ✅ Approve and merge - this is solid work that enhances the module's capabilities significantly. 🏁 Script executed: Length of output: 4580 Perfect! After examining all the files, I can confirm this is a comprehensive and well-executed implementation. Here's my complete assessment: ✅ Implementation Quality - ExcellentCode Structure & Terraform Best Practices:
Validation & Safety:
Documentation & Examples:
Testing:
🎯 AWS SSO Integration ReadyYour implementation perfectly supports the stated goal of enabling SAML SSO between Google Workspace and AWS. The examples show realistic IAM role ARNs and SAML provider configurations that will work in production. 📝 Very Minor ObservationThere's a small typo in the test file: schema_name = "Artibitrarily_Data" # Should be "Arbitrary_Data"But this doesn't affect functionality at all. 🚀 Final Recommendation✅ LGTM - Ready to merge! This is a production-ready implementation that:
Excellent work on this feature! 🎉 |
🤖 I have created a release *beep* *boop* --- ## [0.2.0](v0.1.0...v0.2.0) (2025-06-06) ### Features * add user custom schemas ([#6](#6)) ([4f47941](4f47941)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Weston Platter <weston.platter@masterpoint.io>
what
custom_schemastovar.usersandgoogleworkspace_user.defaultsSummary by CodeRabbit