Repository navigation
feat(secretmanager): Add Cloud SQL managed-rotation samples - #14562
suvidha-malaviya wants to merge 8 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces new regional samples and corresponding tests for Google Cloud Secret Manager, specifically covering creating secrets with Cloud SQL credentials, enabling managed rotation, and triggering rotation. Additionally, the google-cloud-secret-manager dependency is updated to version 2.30.0 in requirements.txt. However, there are critical runtime issues in both enable_regional_secret_managed_rotation.py and rotate_regional_secret.py where the API request payloads incorrectly use the key parent instead of name for the secret's resource name, which will result in a ValueError at runtime.
|
Here is the summary of changes. You are about to add 7 region tags.
This comment is generated by snippet-bot.
|
| # See the License for the specific language governing permissions and | ||
| """ | ||
| command line application and sample code for creating a new secret that is | ||
| eligible for Cloud SQL managed rotation. |
There was a problem hiding this comment.
lets update it to reflect the message better, something like:
| eligible for Cloud SQL managed rotation. | |
| command line application and sample code for creating a new secret with type CLOUD_SQL_DB_CREDENTIALS, eligible for managed rotation. |
| # This built-in identity is what you grant Cloud SQL IAM permissions to, | ||
| # so that Secret Manager can rotate the database password on its behalf. | ||
| print( | ||
| "Grant this identity Cloud SQL IAM permissions to enable rotation: " |
There was a problem hiding this comment.
CLOUD SQL User rotate IAM permissions to enable managed rotation
| # See the License for the specific language governing permissions and | ||
| """ | ||
| command line application and sample code for enabling managed rotation of | ||
| a Cloud SQL DB credentials secret. |
There was a problem hiding this comment.
| a Cloud SQL DB credentials secret. | |
| command line application and sample code to enable managed rotation of a CLOUD_SQL_DB_CREDENTIALS typed secret |
| Enable managed rotation for a Cloud SQL DB credentials secret. This | ||
| links the secret to a Cloud SQL instance and database user, and can | ||
| only be called once per secret. It adds the secret's first version and | ||
| sets the matching password on the Cloud SQL user, taking the place of |
There was a problem hiding this comment.
This is confusing, please update it accordingly to the below suggestion.
| sets the matching password on the Cloud SQL user, taking the place of | |
| It validates and enables the rotation, adding a version and sets the passed password (optional). | |
| Note: AddSecretVersion is disabled on the CLOUD_SQL_DB_CREDENTIALS currently and for any necesary manual rotations please trigger rotate_secret |
|
|
||
| instance_id is the bare Cloud SQL instance ID (e.g. "my-instance") -- | ||
| not a connection name. Neither the project nor the region should be | ||
| included: passing "PROJECT_ID:INSTANCE_ID" (as gcloud's own |
There was a problem hiding this comment.
Lets not add this, I have created a bug to resolve this; technically we should resolve the error in gcloud rather than adding it in the documentation. Please let us know if you find any similar issues in the future
| # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| # See the License for the specific language governing permissions and | ||
| """ | ||
| command line application and sample code for triggering a managed |
There was a problem hiding this comment.
Lets change it to triggering an adhoc rotation for the managed CLOUD_SQL_DB_CREDENTIALS typed secret
| rotation_period_seconds: int, | ||
| ) -> secretmanager_v1.Secret: | ||
| """ | ||
| Reconfigure the recurring rotation schedule on a secret that already |
There was a problem hiding this comment.
On the update_regional_secret_with_managed_rotation_schedule sample across the 7 PRs, could we make two quick comment cleanups inside the [START secretmanager_update_regional_secret_with_managed_rotation_schedule] block?
Clarify when schedule updates are allowed: On the backend, setting rotation.next_rotation_time and rotation.rotation_period on a CLOUD_SQL_DB_CREDENTIALS secret does NOT require EnableManagedRotation to be called first (customers can configure the rotation schedule before or after enabling managed rotation). Also, UpdateSecret with rotation works on other secret types too if Pub/Sub topics are configured — what's unique to CLOUD_SQL_DB_CREDENTIALS is that Pub/Sub topics aren't required.**
There was a problem hiding this comment.
Yes updated
| }, | ||
| } | ||
|
|
||
| # Mask only the two subfields being set here, not the whole "rotation" |
There was a problem hiding this comment.
Remove internal testing notes from the update_mask comment: Since everything inside [START ...] / [END ...] is rendered verbatim on cloud.google.com, could we drop the parentheticals (confirmed empirically against a live project) (in Python/Go/PHP/Ruby) and (the same behavior confirmed against a live project in this same port's Go samples) (in Java/Node.js)?
| secret_type: secretmanager.Secret.SecretType, | ||
| ) -> secretmanager.Secret: | ||
| """ | ||
| Create a new secret with the given secret type restriction (e.g. |
There was a problem hiding this comment.
This is not required, lets just add that CLOUD_SQL_DB_CREDENTIALS is only supported in the regional secret
…ation samples Align docstrings and comments with other samples and clarify rotation schedule behavior.
Added samples for Secret Manager's Cloud SQL managed-rotation feature (regional secrets only — this feature isn't available for global secrets)
Also added two global scenario with secret-type:
Added test coverage for all of the above:
test_create_regional_secret_with_cloud_sql_credentials,test_enable_regional_secret_managed_rotation,test_rotate_regional_secret,test_update_regional_secret_with_managed_rotation_schedule,test_get_regional_secret_type,test_create_secret_with_type,test_get_secret_typeNote: requires google-cloud-secret-manager>=2.30.0, which itself requires Python>=3.10 — this feature does not exist in 2.29.0 or earlier.
Checklist
nox -s py-3.9(see Test Environment Setup)nox -s lint(see Test Environment Setup)-
CLOUD_SQL_INSTANCE/CLOUD_SQL_USER— a pre-provisioned, long-lived Cloud SQL instance + DB user for managed-rotation tests to point at (same pattern as this repo's other Cloud SQL-backed sample tests)- The identity running these tests additionally needs
resourcemanager.projects.getIamPolicy/setIamPolicyon the test project (e.g.roles/resourcemanager.projectIamAdmin)