Fix/smart records merge on create #52
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When user creates a resource with the records that already exist, the creating process should correctly merge it without duplicates Signed-off-by: Ihor Levchenko <[email protected]>
Signed-off-by: Ihor Levchenko <[email protected]>
Also commit contains: - improved errors handling - resolved an error upon generating a plan Signed-off-by: Ihor Levchenko <[email protected]>
Signed-off-by: Ihor Levchenko <[email protected]>
StyleT
suggested changes
Jul 16, 2021
docs/resources/domain_records.md
Outdated
| @@ -60,3 +60,5 @@ resource "namecheap_domain_records" "my-domain2-com" { | |||
| - `type` - (Required) Possible values: A, AAAA, ALIAS, CAA, CNAME, MX, MXE, NS, TXT, URL, URL301, FRAME | |||
| - `mx_pref` - (Optional) MX preference for host. Applicable for MX records only | |||
| - `ttl` - (Optional) Time to live for all record types. Possible values: any value between 60 to 60000 | |||
|
|
|||
| ~> It is strongly recommended to set `address` and `hostname` in lower case to prevent undefined behavior! | |||
There was a problem hiding this comment.
Can we always convert them to lowercase automatically during comparison?
There was a problem hiding this comment.
Basically, yes.
However, the existing behavior works in the way where if a user changes the camel case of the address of any record, the provider will "re-create" the record. I will have to make the READ smarter.
I will revert on this in the context of a current PR.
| @@ -34,6 +34,9 @@ The same workflow works for both: `record` items and `nameservers`. | |||
|
|
|||
| -> This is the default behavior, however, we recommend to set the mode explicitly. | |||
|
|
|||
| ~> Upon creating a new resource with the records or nameservers that already exist, the provider will throw a duplicate | |||
There was a problem hiding this comment.
I think we need to add that it happens if subdomain & record type match
There was a problem hiding this comment.
Noted. I will check this.
At the moment, we have duplicates comparison only by hostname (subdomain), type, and address.
However, user allowed to create "mistakes" like:
resource "namecheap_domain_records" "domain" {
#...
record {
hostname = "dev"
type = "A"
address = "11.22.33.44"
}
record {
hostname = "dev"
type = "A"
address = "33.33.33.33"
}
}I will amend this behavior (and provide tests for all cases! 😆 )
Signed-off-by: Ihor Levchenko <[email protected]>
Signed-off-by: Ihor Levchenko <[email protected]>
ifree92
force-pushed
the
fix/smartRecordsMergeOnCreate
branch
18 times, most recently
from
b282dea to
5a3ce90
Compare
ifree92
force-pushed
the
fix/smartRecordsMergeOnCreate
branch
from
5a3ce90 to
b8c448a
Compare
Signed-off-by: Ihor Levchenko <[email protected]>
ifree92
force-pushed
the
fix/smartRecordsMergeOnCreate
branch
from
b8c448a to
b7c8471
Compare
Signed-off-by: Ihor Levchenko <[email protected]>
StyleT
reviewed
Jul 22, 2021
.github/workflows/ci.yml
Outdated
| source go-env.sh | ||
| make testacc | ||
| env: | ||
| NAMECHEAP_USER_NAME: ${{ secrets.NAMECHEAP_USER_NAME }} |
There was a problem hiding this comment.
Let's move "non-secret" var values to this file. It will simplify debugging
ifree92
force-pushed
the
fix/smartRecordsMergeOnCreate
branch
2 times, most recently
from
4f0bb25 to
f56564c
Compare
Signed-off-by: Ihor Levchenko <[email protected]>
ifree92
force-pushed
the
fix/smartRecordsMergeOnCreate
branch
from
f56564c to
68b3406
Compare
StyleT
approved these changes
Jul 23, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.