Skip to content
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

Fix records that has the same FQDN #76

Merged
merged 2 commits into from
Jan 6, 2021
Merged

Fix records that has the same FQDN #76

merged 2 commits into from
Jan 6, 2021

Conversation

wbeuil
Copy link
Contributor

@wbeuil wbeuil commented Jan 6, 2021

Q A
πŸ› Bug fix? yes
πŸš€ New feature? no
⚠ Deprecations? no
❌ BC Break no
πŸ”— Related issues #74
❓ Documentation no

Description

For two records that has the same fully qualified domain name, our diff would messed up the match between them since two records could have the same FQDN but not the same type (TXT, A ...).

We decided to normalise the state resource by modifying its id in order to match what we have from the AWS provider. Thus we must have an Id of type ZoneId_FQDN_Type_SetIdentifier.

@wbeuil wbeuil requested a review from eliecharra January 6, 2021 11:58
@wbeuil wbeuil self-assigned this Jan 6, 2021
@wbeuil wbeuil requested a review from a team as a code owner January 6, 2021 11:58
@eliecharra eliecharra added the kind/bug Something isn't working label Jan 6, 2021
@eliecharra eliecharra added this to the v0.2.1 milestone Jan 6, 2021
@codecov
Copy link

codecov bot commented Jan 6, 2021

Codecov Report

Merging #76 (7d6b6a8) into main (063ff82) will increase coverage by 0.00%.
The diff coverage is 75.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main      #76   +/-   ##
=======================================
  Coverage   67.03%   67.04%           
=======================================
  Files         146      146           
  Lines        3307     3314    +7     
=======================================
+ Hits         2217     2222    +5     
- Misses        841      842    +1     
- Partials      249      250    +1     
Impacted Files Coverage Ξ”
pkg/resource/aws/aws_route53_record_ext.go 80.00% <71.42%> (-7.50%) ⬇️
pkg/resource/aws/aws_route53_record.go 100.00% <100.00%> (ΓΈ)

@eliecharra eliecharra merged commit 1abe965 into main Jan 6, 2021
@eliecharra eliecharra deleted the fix/records branch January 6, 2021 13:50
@eliecharra eliecharra linked an issue Jan 6, 2021 that may be closed by this pull request
@eliecharra eliecharra removed this from the v0.2.1 milestone Jan 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Route53 found drifted resources where they really aren't
2 participants