-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Remove IP validation for host of on-premise instance #9225
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
In admin API, we don't have validation for host of on-premise instances. Host can take the value of IP address or DNS address. As of now it is validating that host contain the IP address in terraform, in order to be consistent with admin API and accept DNS address as well, removing the validation function for host.
Hello! I am a robot. It looks like you are a: Community Contributor @SarahFrench, a repository maintainer, has been assigned to review your changes. If you have not received review feedback within 2 business days, please leave a comment on this PR asking them to take a look. You can help make sure that review is quick by doing a self-review and by running impacted tests locally. |
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are. Terraform GA: Diff ( 1 file changed, 4 insertions(+), 5 deletions(-)) |
Tests analyticsTotal tests: Action takenFound 5 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected testsTestAccAlloydbInstance_clientConnectionConfig_update|TestAccAlloydbCluster_secondaryClusterMandatoryFields|TestAccAlloydbBackup_createBackupWithMandatoryFields|TestAccAlloydbCluster_secondaryClusterDefinedSecondaryConfigButMissingClusterTypeSecondary|TestAccAlloydbBackup_alloydbBackupFullExample |
Rerun these tests in REPLAYING mode to catch issues
|
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 reason for removing the validation makes sense to me, but I recommend improving the field's description before merging this change
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are. Terraform GA: Diff ( 2 files changed, 5 insertions(+), 6 deletions(-)) |
Tests analyticsTotal tests: Action takenFound 5 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected testsTestAccAlloydbBackup_createBackupWithMandatoryFields|TestAccAlloydbBackup_alloydbBackupFullExample|TestAccAlloydbCluster_secondaryClusterDefinedSecondaryConfigButMissingClusterTypeSecondary|TestAccAlloydbCluster_secondaryClusterMandatoryFields|TestAccCloudRunService_cloudRunServiceSqlExample |
Rerun these tests in REPLAYING mode to catch issues
|
Note: Test failures are unrelated The |
According to API doc to setup a source representation instance, user can pass the IPv4 address and port for the external server, or the the DNS address for the external server. When user try to pass DNS address in terraform configuration file, they encounter Error: could not parse "host" to IP address.
In admin API, we don't have validation for host of on-premise instances. As of now it is validating that host contain the IP address in terraform. In order to be consistent with admin API and accept DNS address as well, removing the validation function for host.
Release Note Template for Downstream PRs (will be copied)