-
Notifications
You must be signed in to change notification settings - Fork 68
[FEATURE] Added Secrets Manager for GCE #261
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 5 out of 10 changed files in this pull request and generated 2 comments.
Files not reviewed (5)
- datasource/secretsmanager/test-fixtures/template.pkr.hcl: Language not supported
- docs-partials/datasource/secretmanager/Config-not-required.mdx: Language not supported
- docs-partials/datasource/secretmanager/Config-required.mdx: Language not supported
- docs-partials/datasource/secretmanager/DatasourceOutput.mdx: Language not supported
- go.mod: Language not supported
| // The decoded values from this spec will then be applied to a FlatConfig. | ||
| func (*FlatConfig) HCL2Spec() map[string]hcldec.Spec { | ||
| s := map[string]hcldec.Spec{ | ||
| "project_id": &hcldec.AttrSpec{Name: "project_id", Type: cty.String, Required: false}, |
Copilot
AI
Apr 9, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Config struct marks 'project_id' as required but the HCL2 spec sets it as not required. Consider setting Required to true in the spec to match the Config validation.
| "project_id": &hcldec.AttrSpec{Name: "project_id", Type: cty.String, Required: false}, | |
| "project_id": &hcldec.AttrSpec{Name: "project_id", Type: cty.String, Required: true}, |
| func (*FlatConfig) HCL2Spec() map[string]hcldec.Spec { | ||
| s := map[string]hcldec.Spec{ | ||
| "project_id": &hcldec.AttrSpec{Name: "project_id", Type: cty.String, Required: false}, | ||
| "name": &hcldec.AttrSpec{Name: "name", Type: cty.String, Required: false}, |
Copilot
AI
Apr 9, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similarly, 'name' is required in the Config struct but not marked as such in the HCL2 spec. Aligning this by setting Required to true can improve configuration validation.
| "name": &hcldec.AttrSpec{Name: "name", Type: cty.String, Required: false}, | |
| "name": &hcldec.AttrSpec{Name: "name", Type: cty.String, Required: true}, |
6440ead to
dbff7fc
Compare
anshulsharma-hashicorp
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for fetching secrets from Google Cloud’s Secret Manager by introducing a new datasource.
- Registers the new "secretsmanager" datasource in main.go
- Implements secrets manager configuration, execution, and output logic in datasource/secretsmanager/data.go
- Adds both unit and acceptance tests to validate behavior for the new secrets manager feature
Reviewed Changes
Copilot reviewed 6 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| main.go | Registers the new secretsmanager datasource |
| datasource/secretsmanager/data.go | Implements secrets manager datasource |
| datasource/secretsmanager/data_test.go | Adds unit tests for configuration errors |
| datasource/secretsmanager/data_acc_test.go | Adds acceptance tests for live GCP integration |
| datasource/secretsmanager/data.hcl2spec.go | Auto-generated HCL2 spec mapping for config |
Files not reviewed (5)
- datasource/secretsmanager/test-fixtures/template.pkr.hcl: Language not supported
- docs-partials/datasource/secretsmanager/Config-not-required.mdx: Language not supported
- docs-partials/datasource/secretsmanager/Config-required.mdx: Language not supported
- docs-partials/datasource/secretsmanager/DatasourceOutput.mdx: Language not supported
- go.mod: Language not supported
This PR adds support for the secrets manager from google cloud.
This implementation is in continuation from the original PR #99
This change allows the user to fetch the secrets from their secret manager in GCP, with an option for specifying:
Closes #29