Skip to content

feat: Add support for verifying Cloud SQL instances#1681

Closed
Amit2465 wants to merge 1 commit intogruntwork-io:mainfrom
Amit2465:ay/feat-add-gcp-cloudsql-support
Closed

feat: Add support for verifying Cloud SQL instances#1681
Amit2465 wants to merge 1 commit intogruntwork-io:mainfrom
Amit2465:ay/feat-add-gcp-cloudsql-support

Conversation

@Amit2465
Copy link
Copy Markdown
Contributor

@Amit2465 Amit2465 commented Mar 9, 2026

Description

This PR adds support for Google Cloud SQL to the GCP module. It introduces helper functions to verify the existence of Cloud SQL instances and retrieve their database version, along with a complete Terraform example and automated integration tests covering both MySQL and PostgreSQL engines.

This is a new feature addition and does not introduce any backward-incompatible changes.

TODOs

Read the Gruntwork contribution guidelines.

  • Update the docs.
  • Run the relevant tests successfully, including pre-commit checks.
  • Ensure any 3rd party code adheres with our license policy or delete this line if its not applicable.
  • Include release notes. If this PR is backward incompatible, include a migration guide.
  • Make a plan for release of the functionality in this PR. If it delivers value to an end user, you are responsible for ensuring it is released promptly, and correctly. If you are not a maintainer, you are responsible for finding a maintainer to do this for you.

Release Notes (draft)

Added support for verifying Google Cloud SQL instances in the GCP module, including database engine version validation for MySQL and PostgreSQL.

Migration Guide

No backward incompatible changes were introduced.


Verification Results

I have verified this PR locally against a real GCP project:

  • Linting: Passed using golangci-lint run.
  • Formatting: Passed go fmt and terraform fmt.
  • Integration Test: Successfully ran TestTerraformGcpCloudSQLExample covering both MySQL and PostgreSQL engines in parallel.

Test Output Snippet:

=== RUN   TestTerraformGcpCloudSQLExample
=== RUN   TestTerraformGcpCloudSQLExample/mysql
=== RUN   TestTerraformGcpCloudSQLExample/postgres
=== CONT  TestTerraformGcpCloudSQLExample/mysql
=== CONT  TestTerraformGcpCloudSQLExample/postgres
...
TestTerraformGcpCloudSQLExample/postgres 2026-03-09T22:22:07 cloudsql.go:22: Verifying Cloud SQL instance terratest-cloudsql-6k3njo exists in project amith-1772452782257
TestTerraformGcpCloudSQLExample/postgres 2026-03-09T22:22:08 cloudsql.go:39: Getting database version of Cloud SQL instance terratest-cloudsql-6k3njo in project amith-1772452782257
...
TestTerraformGcpCloudSQLExample/mysql 2026-03-09T22:26:27 cloudsql.go:22: Verifying Cloud SQL instance terratest-cloudsql-vloz53 exists in project amith-1772452782257
TestTerraformGcpCloudSQLExample/mysql 2026-03-09T22:26:28 cloudsql.go:39: Getting database version of Cloud SQL instance terratest-cloudsql-vloz53 in project amith-1772452782257
...
--- PASS: TestTerraformGcpCloudSQLExample/postgres (630.55s)
--- PASS: TestTerraformGcpCloudSQLExample/mysql (921.10s)
PASS

@james00012
Copy link
Copy Markdown
Contributor

One suggestion: the error message in getCloudSQLInstanceE (line 60) wraps the API error with "does not exist", but the underlying error could also be an auth or network issue. Consider rewording to something like "failed to get Cloud SQL instance %s in project %s: %v" so it doesn't mask the real cause. The strings.ToLower on random.UniqueId() is a no-op and can be dropped. Otherwise this looks good to merge.

@Amit2465 Amit2465 force-pushed the ay/feat-add-gcp-cloudsql-support branch from 00dd4d4 to 098f6a5 Compare March 25, 2026 03:49
@Amit2465
Copy link
Copy Markdown
Contributor Author

Fixed both: updated error message to not mask the real cause, and dropped the no-op strings.ToLower

@james00012
Copy link
Copy Markdown
Contributor

Thanks for the contribution! The code is clean, tests are thorough, and the overall structure follows existing module conventions well. A few things to address before merging:

Context variants (main ask): The GCP module has been migrating to context-aware function signatures (see storage.go, compute.go, oslogin.go for examples). Since this is new code with no existing callers, we can skip the deprecated non-context versions entirely and just ship the Context variants directly:

func AssertCloudSQLInstanceExistsContext(t testing.TestingT, ctx context.Context, projectID, instanceName string)
func AssertCloudSQLInstanceExistsContextE(t testing.TestingT, ctx context.Context, projectID, instanceName string) error

Same for GetCloudSQLInstanceDatabaseVersion. The internal helpers (getCloudSQLInstanceE, newCloudSQLService) should accept a context.Context param instead of creating context.Background() internally.

Minor: In cloudsql.go:60, use %w instead of %v for error wrapping so callers can use errors.Is/errors.As:

return nil, fmt.Errorf("failed to get Cloud SQL instance %s in project %s: %w", instanceName, projectID, err)

Housekeeping: Please rebase/update your branch against main to pick up the latest changes.

Everything else — naming, error handling, logging, test structure, Terraform example — looks great.

@Amit2465 Amit2465 force-pushed the ay/feat-add-gcp-cloudsql-support branch from 098f6a5 to 66fb80b Compare March 30, 2026 04:59
This commit adds helper functions to the `gcp` module to enable
automated testing of Cloud SQL instances using the Cloud SQL Admin API.

As part of this:
* Add new `AssertCloudSQLInstanceExists` and `AssertCloudSQLInstanceExistsE` functions
* Add new `GetCloudSQLInstanceDatabaseVersion` and `GetCloudSQLInstanceDatabaseVersionE` functions
* Add Terraform example fixture in `examples/terraform-gcp-cloudsql-example`
* Add integration tests covering MySQL and PostgreSQL engines
@Amit2465 Amit2465 force-pushed the ay/feat-add-gcp-cloudsql-support branch from 66fb80b to a3e1dac Compare March 30, 2026 05:50
@Amit2465
Copy link
Copy Markdown
Contributor Author

Thanks for the detailed review! Here's what was addressed:

  • Context variants: Replaced all functions with Context variants only (AssertCloudSQLInstanceExistsContext, AssertCloudSQLInstanceExistsContextE, GetCloudSQLInstanceDatabaseVersionContext, GetCloudSQLInstanceDatabaseVersionContextE). Internal helpers (getCloudSQLInstanceE, newCloudSQLService) now accept context.Context instead of creating context.Background() internally.
  • %w error wrapping: Updated all error formatting to use %w for proper errors.Is/errors.As support.
  • Rebase: Branch has been rebased against main.

Also added modules/gcp/cloudsql_test.go to match the testing pattern of other GCP modules, covering both positive and negative cases.

@james00012
Copy link
Copy Markdown
Contributor

Thanks for addressing all the feedback! This is nearly ready. A few final items:

  1. Hard-coded tier: CreateCloudSQLInstanceContextE hard-codes "db-f1-micro", which isn't valid for all engines. Add a tier parameter or accept *sqladmin.Settings.
  2. go.sum update: The diff doesn't include go.mod/go.sum changes — please run go mod tidy and include the result.
  3. Remove tt := tt: Unnecessary with Go 1.22+ (we're on 1.26).

Everything else looks great — clean code, good test coverage, follows module conventions well.

@james00012
Copy link
Copy Markdown
Contributor

Hi @Amit2465, closing this PR as there were no response for the past week. Feel free to reopen this when you have the capacity to continue making changes on this PR!

@james00012 james00012 closed this Apr 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants