-
Notifications
You must be signed in to change notification settings - Fork 101
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
Validate private network for cloudsql is in expected format #374
Validate private network for cloudsql is in expected format #374
Conversation
@@ -349,7 +349,7 @@ type IPConfiguration struct { | |||
|
|||
// PrivateNetwork: The resource link for the VPC network from which the | |||
// Cloud SQL instance is accessible for private IP. For example, | |||
// /projects/myProject/global/networks/default. This setting can be updated, |
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.
This is indeed the same comment as the one gcloud API: https://cloud.google.com/sql/docs/mysql/admin-api/rest/v1beta4/instances#IpConfiguration
However, using this format (with leading /) does not work actually:
create failed: cannot create new CloudSQL instance: googleapi: Error 400: Invalid request: Project redacted-project has invalid private network name /projects/redacted-project/global/networks/default., invalid
pkg/controller/database/cloudsql.go
Outdated
|
||
type specValidator struct{} | ||
|
||
// Initialize adds the external tags to spec.forProvider.settings.userLabels |
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.
Looks like this may have been copied over from tags initializer
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.
Fixed 👍
d13e5d0
to
7f56e6d
Compare
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.
@turkenh would it be possible to use the CRD field Pattern
validation for this? https://book.kubebuilder.io/reference/markers/crd-validation.html
Closes crossplane-contrib#352 Signed-off-by: Hasan Turken <[email protected]>
7f56e6d
to
56aee82
Compare
Definitely, Thanks for pointing out! |
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.
Nice! Thanks @turkenh :)
Description of your changes
Google Cloud API expects the resource link for the VPC network for the CloudSQL private network field. However, it also accepts VPC name (e.g. "default") but looks like it internally converts it to resource link by assuming it is in the same project, (projects/[project]/global/networks/[name]). This is causing issues like #352 where users are not aware about the underlying problem.
This PR adds a pattern validation on CRD using kubebuilder marker (thanks @hasheddan for pointing that out).
Fixes #352
I have:
make reviewable test
to ensure this PR is ready for review.How has this code been tested